Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update EIP-7201: Move to Last Call #7519

Closed
wants to merge 21 commits into from
Closed

Update EIP-7201: Move to Last Call #7519

wants to merge 21 commits into from

Conversation

frangio
Copy link
Contributor

@frangio frangio commented Aug 23, 2023

No description provided.

@github-actions github-actions bot added c-update Modifies an existing proposal s-draft This EIP is a Draft t-erc labels Aug 23, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Aug 23, 2023 •
edited

✅ All reviewers have approved.

@frangio frangio changed the title Update EIP-7201: Add 256 alignment for Verkle migration and other changes Update EIP-7201: Add 256 alignment and other changes, and move to Last Call Aug 23, 2023
@github-actions github-actions bot added c-status Changes a proposal's status and removed c-update Modifies an existing proposal s-draft This EIP is a Draft labels Aug 24, 2023
@eth-bot eth-bot changed the title Update EIP-7201: Add 256 alignment and other changes, and move to Last Call Update EIP-7201: Move to Last Call Aug 24, 2023
@eth-bot eth-bot added the e-review Waiting on editor to review label Aug 24, 2023
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Aug 24, 2023
@github-actions
Copy link

The commit 9becbc1 (as a parent of 5207003) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Aug 24, 2023
@frangio frangio marked this pull request as ready for review August 24, 2023 19:43
@frangio frangio requested a review from eth-bot as a code owner August 24, 2023 19:43
@frangio
Copy link
Contributor Author

frangio commented Aug 25, 2023

This is ready for review @axic, @Pandapip1, @SamWilsn, @xinbenlv

@frangio
Copy link
Contributor Author

frangio commented Aug 28, 2023

Merged changes separately in #7529. Leaving this PR for the move to Last Call only.

@poojaranjan poojaranjan mentioned this pull request Sep 5, 2023
Closed
21 tasks
SamWilsn
SamWilsn requested changes Sep 5, 2023
View reviewed changes
Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to complete the security considerations section before leaving draft.

EIPS/eip-7201.md Outdated
Comment on lines 7 to 8
status: Last Call
last-call-deadline: 2023-09-15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally proposals go from Draft to Review, then to Last Call. Any particular reason you're skipping Review?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EIP has been effectively in Review for months now. I recognize we should've formalized that in the status but I only realized once we wanted to move to Last Call.

Pandapip1
Pandapip1 requested changes Sep 7, 2023
View reviewed changes
Copy link
Member

@Pandapip1 Pandapip1 left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be okay with moving from Draft to Last Call, but only if the deadline is a full month long.

EIPS/eip-7201.md Outdated
@@ -4,7 +4,8 @@ title: Namespaced Storage Layout
description: A formula for the storage location of structs in the namespaced storage pattern.
author: Francisco Giordano (@frangio), Hadrien Croubois (@Amxx), Ernesto García (@ernestognw), Eric Lau (@ericglau)
discussions-to: https://ethereum-magicians.org/t/eip-7201-namespaced-storage-layout/14796
status: Draft
status: Last Call
last-call-deadline: 2023-09-15
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
last-call-deadline: 2023-09-15
last-call-deadline: 2023-10-7

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've actively requested and received feedback for a while already as seen in the discussions thread.

We're going to release code using this EIP at the beginning of October so it will become de facto Final at that point, since as authors we're not going to accept normative changes after the code is released. I'll set the last call deadline to Oct 1st to match this reality.

Pandapip1
Pandapip1 previously approved these changes Sep 12, 2023
View reviewed changes
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with this irregular state transition given the longer last call deadline.

@eth-bot eth-bot enabled auto-merge (squash) September 12, 2023 16:21
eth-bot
eth-bot approved these changes Sep 12, 2023
View reviewed changes
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Reviewers Have Approved; Performing Automatic Merge...

@frangio
Copy link
Contributor Author

frangio commented Sep 13, 2023

Is there an issue with the auto merge?

@mudgen
Copy link
Contributor

mudgen commented Sep 15, 2023

I made a new comment about the standard here: https://ethereum-magicians.org/t/eip-7201-namespaced-storage-layout/14796/11?u=mudgen

@frangio
Copy link
Contributor Author

frangio commented Sep 19, 2023

@Pandapip1 Can you take a look at the auto merge?

@xinbenlv
Copy link
Contributor

Agree it's a pain. For the moment can you submit a PR to move to Review first, and then we can merge the next PR to move it last call?

eth-bot
eth-bot previously approved these changes Sep 22, 2023
View reviewed changes
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Reviewers Have Approved; Performing Automatic Merge...

@frangio frangio mentioned this pull request Sep 22, 2023
Merged
@frangio frangio dismissed stale reviews from eth-bot and Pandapip1 September 23, 2023 03:00

The merge-base changed after approval.

@frangio
Copy link
Contributor Author

frangio commented Sep 23, 2023

Before this can be merged @SamWilsn needs to approve the requested changes.

The security section that was requested was added in #7760.

xinbenlv
xinbenlv reviewed Sep 25, 2023
View reviewed changes
EIPS/eip-7201.md
@@ -4,7 +4,8 @@ title: Namespaced Storage Layout
description: A formula for the storage location of structs in the namespaced storage pattern.
author: Francisco Giordano (@frangio), Hadrien Croubois (@Amxx), Ernesto García (@ernestognw), Eric Lau (@ericglau)
discussions-to: https://ethereum-magicians.org/t/eip-7201-namespaced-storage-layout/14796
status: Review
status: Last Call
last-call-deadline: 2023-10-01
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
last-call-deadline: 2023-10-01
last-call-deadline: 2023-10-09

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned earlier (#7519 (comment)) I didn't want to set the deadline beyond Oct 1st because we're doing a release that day and after that point as authors we won't accept normative changes to the EIP.

xinbenlv
xinbenlv approved these changes Sep 25, 2023
View reviewed changes
Copy link
Contributor

@xinbenlv xinbenlv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Nit-pick: please don't forget to update. the Last Call Deadline.
If the merge was triggered immediately, please don't forget to create another PR to update the last call deadline date :)

eth-bot
eth-bot approved these changes Sep 25, 2023
View reviewed changes
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Reviewers Have Approved; Performing Automatic Merge...

eth-bot
eth-bot approved these changes Sep 25, 2023
View reviewed changes
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Reviewers Have Approved; Performing Automatic Merge...

@frangio frangio closed this Sep 25, 2023
auto-merge was automatically disabled September 25, 2023 21:02

Pull request was closed

@frangio frangio mentioned this pull request Sep 25, 2023
Merged
@frangio
Copy link
Contributor Author

frangio commented Sep 25, 2023

Closed in favor of #7779.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xinbenlv xinbenlv xinbenlv approved these changes

@eth-bot eth-bot eth-bot approved these changes

@Pandapip1 Pandapip1 Pandapip1 left review comments

@SamWilsn SamWilsn Awaiting requested review from SamWilsn

Assignees
No one assigned
Labels
c-status Changes a proposal's status e-review Waiting on editor to review t-erc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@frangio @eth-bot @mudgen @xinbenlv @Pandapip1 @SamWilsn @Amxx